-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[김창희] sprint2 #39
The head ref may contain hidden characters: "Basic-\uAE40\uCC3D\uD76C-sprint2"
[김창희] sprint2 #39
Conversation
첫 번째 커밋
디렉토리 구조 수정
1. 파비콘 삽입 2. 자바스크립트 Fetch API를 이용하여 Html파일안에 다른 HTML파일을 불러오고, 불러온 HTML파일이 Header.html, Footer.html와 같이 특정 컴포넌트 HTML 파일을 불러와야한다면, 그것을 불러오게끔 JS 코드 구축. 즉, 완전한 리액트식 분리형 컴포넌트 구조 구축 3. 리액트식 컴포넌트 구조 도입에 따른 디렉토리, 파일 구조 수정 4. 페이지별 title 및 링크 오류 수정 5. button 태그 안 a 태그 -> a 태그 안 button 태그 수정 6. 중복 이미지 파일 삭제 # 현재 발생되는 주의사항 Failed to decode downloaded font:
1. Header position을 sticky로 변경 2. login 페이지 코드 구현 3. 오류 사항 해결 - 현재 페이지 HTML 파일에서 컴포넌트 HTML 파일을 불러올 때, .outerHTML = data 형식을 취하는데, 보안상 data라는 string 형식에 <script> 코드가 있을시 해당 코드를 단순 string 취급한다. 따라서 <script> 적용 문제해결을 위해 data 변수의 string 형식에서 <script>의 text나 src의 내용을 찾아 다른 변수에 저장 후, createElement('script')를 통해 script를 따로 추가를 해줌 4. Home.html의 label을 h5로 수정 5. Footer.html의 li 태그 안 a 태그 -> a 태그 안 li 태그
1. Login.html 눈 아이콘 추가, 아이콘 토글 관련 기능 JS 코드 작성 2. signup 페이지 관련 html 파일들 구현 및 JS 코드 작성
나중에 시도해볼 사항 :
|
src 디렉토리 하위에 있었던 index.html 파일을 최상위 디렉토리 ('/')로 옮겨서 루트 경로가 'src/' 로 시작하는 것이 아닌 '/' 로 시작하게 함
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
창희님 이주차 미션 고생하셨습니다.
재사용을 고민하시고 새롬게 시도해보신것이 좋았습니다.
이런 경우 web component로 검색해보시면 작업하시는데 좋을 것 같습니다.
https://developer.mozilla.org/en-US/docs/Web/API/Web_components#custom_elements
https://developer.mozilla.org/en-US/docs/Web/API/Web_components/Using_custom_elements
- 동일한 내용에 대해서는 코드에서 한번만 코멘트를 답니다.
- commit message를 적으실때 어떤 변경인지 알 수 있도록 해주시면 좋겠습니다.
- rem을 쓰고 있는데 html font-size를 10으로 고정을 시키면 어떻게 반응형으로 수정해야할지 모르겠습니다. 데스크탑은 p태그가 24px, 태블릿은 18px, 모바일은 16px 기준입니다. 미디어 쿼리로 다 따로 px로 지정을 해줘야하나요? 그렇다면 스프린트 1 미션은 아직 미디어 쿼리를 배우기 전인데 반응형 미션이 있습니다. => 어떻게 반응형으로 수정해야 하냐는 질문에 대해 잘 감이 오지 않습니다. 예시로 들어주신 것에 대해 답변을 드리자면 html font-size: 10px 일 경우 p 태그는 PC 2.4rem, Tablet 1.8rem, Mob 1.6rem과 같은 식으로 입력해주시면됩니다. 만약 반응형작업시 화면 사이즈 별로 규칙이 있으면 기준 font-size를 변경해주셔도 되고, 불규칙하게 변경되면 일일히 변경해주셔야 합니다. 또한 아직 배우기전인 부분이라 심화요구사항에 있는 점 참고해주세요.
<label class="label-font" for="email-tag">이메일</label> | ||
<div class="input-content"> | ||
<input class="email-tag" autocomplete="off" id="email-tag" type="email" placeholder="이메일을 입력해주세요" | ||
required> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2:
자동완성 기능 꺼주시고 라벨과 인풋을 연결하신거 좋습니다.
다만 focus시 영역이 인풋보다 작은 것 같아 가능하시면 수정되면 좋을 것 같습니다.
<a href="https://www.google.com/" target="_blank" rel="noopener noreferrer"> | ||
<li | ||
style="display: flex; justify-content: center; align-items: center; width: 42px; height: 42px; background: white; border-radius: 9999px; border: 1px #F9FAFB"> | ||
<img src="/src/assets/img/ic_google.png" alt="구글 로그인"> | ||
</li> | ||
</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3:
unordered list 의 list item 태그인 li는 ul의 바로 자식으로 있는 것이 더 가독성에 좋을 것 같습니다.
<a href="https://www.google.com/" target="_blank" rel="noopener noreferrer"> | |
<li | |
style="display: flex; justify-content: center; align-items: center; width: 42px; height: 42px; background: white; border-radius: 9999px; border: 1px #F9FAFB"> | |
<img src="/src/assets/img/ic_google.png" alt="구글 로그인"> | |
</li> | |
</a> | |
<li | |
style="display: flex; justify-content: center; align-items: center; width: 42px; height: 42px; background: white; border-radius: 9999px; border: 1px #F9FAFB"> | |
<a href="https://www.google.com/" target="_blank" rel="noopener noreferrer"> | |
<img src="/src/assets/img/ic_google.png" alt="구글 로그인"> | |
</a> | |
</li> |
.then(data => { | ||
element.outerHTML = data; | ||
const loadEvent = new Event('load'); // Fetch로 삽입된 HTML파일은 load 이벤트를 발생시키지 않음 | ||
element.addEventListener('load', () => { // IncludeComponent.js의 일부코드 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3:
주석달아주신 것처럼 IncludeComponent.js와 비슷한 로직이 반복된다면
이를 함수로 빼서 두 파일에서 중복코드를 줄이시면 좋겠네요.
@@ -0,0 +1,39 @@ | |||
window.addEventListener('load', () => { | |||
let allElements = document.getElementsByTagName('*'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2:
let allElements = document.getElementsByTagName('*'); | |
const allElements = document.getElementsByTagName('*'); |
const passwordInput = document.querySelector('.password-tag'); | ||
const passwordIcon = document.querySelector('.password-icon'); | ||
|
||
const checkInputs = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2:
input의 값을 확인(if 문)하고 button disabled 상태를 변경해주는 것이니 이름을 더 적절하게 바꿔주시면 좋겠습니다.
요구사항
기본
심화
(추후 클릭으로 비밀번호를 보거나 가릴 수 있도록 기능을 만들어 갈 예정입니다. -> 해당 기능도 구현완료)
주요 변경사항
앞으로 수정해야할 사항
스크린샷
배포 사이트
https://cheerful-salamander-19dcbf.netlify.app
멘토에게
rem을 쓰고 있는데 html font-size를 10으로 고정을 시키면 어떻게 반응형으로 수정해야할지 모르겠습니다. 데스크탑은 p태그가 24px, 태블릿은 18px, 모바일은 16px 기준입니다. 미디어 쿼리로 다 따로 px로 지정을 해줘야하나요? 그렇다면 스프린트 1 미션은 아직 미디어 쿼리를 배우기 전인데 반응형 미션이 있습니다.
css에서 font-family 중복 사용 제거와 같이, 아직 미작업인 부분이 있습니다. 저번 코멘트의 수정 요청사항은 진행중입니다.